Wait until the desktop ends starting up #231
Conversation
Gnome Shell enables the extensions before the desktop itself is ready to accept widgets. This can result in errors. In the case of appindicator, it results in applications that are launched during startup not showing an indicator because, when the extension tries to add the icon to the Gnome Shell interface, it still isn't ready for that, thus failing. But other applications, launched after the Shell has completed the startup process, will work fine. This patch fixes this by checking in the `enable()` method if Gnome Shell is still starting up, in which case it waits until the `startup-complete` signal is triggered before continuing.
rastersoft
force-pushed
the
wait_until_shell_ends_startup
branch
from
b2aff4b
to
b2f70de
Compare
|
This PR has the opposite effect for me: Without it, I can't reproduce applications not showing their icon but with it, some icons actually disappear. |
|
Nevermind I think it's working correctly, it might have been a combination of an outdated .desktop not starting and also systemd-networkd-wait-online failing. Do you know how this |
|
Nevermind again it is not, with my last reboots I got 3 appindicators shown out of the 5. The 2 not displayed were correctly launched as they appeared in the list of open applications, just no appindicators. |
|
Yes, it is still happening to me, but before I never had the icon; with this patch I have it sometimes. About the startup-complete signal, it is emitted by gnome shell in the js/ui/layout.js, when the startup animation is complete. We had to use it in desktop-icons to ensure that the desktop was painted only after the system was fully ready. |
|
Just a question... Those two apps that don't show, are using libappindicator? Maybe the problem is there... |
|
They are using libappindicator, I can interact with them with the usual menu. |
|
Mmm... The extension returned this error when it failed... I'm going to investigate from here: |
It seems that the reason for not showing sometimes the appindicators during startup is because Gtk.IconTheme.get_default() returns NULL. This seems to happen only very early during startup, so this patch waits until it returns a valid value before continuing enabling the extension. Of course, this patch is just a hack. If this really fixes the problem, the bug must be fixed upstream.
|
I added a second patch to this MR that, during startup, waits until Gtk.IconTheme.get_default() returns a valid value instead of NULL. I rebooted several times and it seems to work. Of course, this is just an ugly hack, but if it really works, then we have something to report in the mutter/gnome-shell repository... |
|
Tried it but does not fix anything for me. Some apps start (the window), but aren't able to get their appindicator in the panel. |
|
Can you spot any error with |
|
Only thing I see is but that might be unrelated. |
|
Mmm... not sure... I was receiving exactly that same error, with also the one of get_for_screen()... |
|
Added another check to manage that case. Can you test it again? |
|
Did a little extra change. Test it now again, please. |
|
Tested it including 6b5bd5c and it seems to behave correctly now :) |
|
Perfect. So there were two problems:
|
rastersoft
force-pushed
the
wait_until_shell_ends_startup
branch
from
6b5bd5c
to
0b1b45f
Compare
|
Squashed the last three commits in a single one. |
|
Just one question: is it important the "theme" parameter? I mean: there seems to be some GLib calls for icons, which won't depend on a Gdk.Screen, but doesn't seem to accept a theme. |
Gdk.DisplayManager has a signal to specify when the default display is available. This patch makes use of it to detect when the enable work can continue. It also does the waiting for the display and for 'startup-complete' in parallel.
|
Ok, now I think we have the right MR: it uses a signal to detect when a display is available, and waits for startup complete in parallel with waiting for the display. What do you think? |
|
Yes, it's working correctly :) |
|
Good! |
|
Hmm new reboot and one out of the 6 appindicators didn't register: antimicroX |
|
Agh!! |
|
With the previous patch did it register? |
| } | ||
|
|
||
| // Ensure that the default Gdk Screen is available | ||
| if (Gtk.IconTheme.get_default() == null) { |
There was a problem hiding this comment.
Maybe here would be better to check !Gdk.DisplayManager.get().get_default_display() instead?
There was a problem hiding this comment.
Gtk.IconTheme.get_default() fails because internally it can't get the default display. In some comments in #mutter they commented that Gtk.IconTheme should not rely on a display (or an screen... now I doubt), because it is an X11 specific structure. So, if in the future, they remove that, Gtk.IconTheme.get_default() will work directly and the extension won't have to wait.
| @@ -403,6 +403,13 @@ class AppIndicators_IconActor extends St.Icon { | |||
| } | |||
|
|
|||
| _createIconByName(path, callback) { | |||
| if (!path) { | |||
| GLib.idle_add(GLib.PRIORITY_DEFAULT_IDLE, () => { | |||
There was a problem hiding this comment.
I'm not sure the idle is needed here, can't we just call the callback and return here?
There was a problem hiding this comment.
Ah, I saw it was failing as per 6b5bd5c
Not sure there's another way to avoid.
In any case, pelase save the idle id in the class and disconnect it on destruction.
There was a problem hiding this comment.
I call it from IDLE to ensure that the calling code ends anything that is doing before the callback is called. Currently it doesn't matter, but being strict, the code that calls _createIconByName() can legally presume that the callback won't be called until the whole function ended. Calling the callback directly instead of using idle_add can result in breaking that assumption.
About the id: the callback already returns false to remove it, so it doesn't need to be disconnected on destruction. In fact, it MUST be removed after the first execution, or the callback would be called every time there is idle time.
(unless I understood the memory management wrong in these cases)
I think not this one specifically but a few hours ago I rebooted and the discord icon looked like this (the 3 dots): I see this in the log this: but I think it was also happening before. |
|
That happens when the panel can't find the icon. That happened to me when, accidentally, I put an incorrect icon name in an indicator. So it can be a bug in the application (setting an incorrect icon name) or a bug in the extension (doesn't search for icons in all possible places). |
|
Hi, can we hope a quick update of KStatusNotifierItem before Gnome Shell 3.38 / Ubuntu 20.10 ? It's boring to have restarting computer 2 times each time to activate the missing icons... Thanks :) |
|
After this being merged, I now have again the opposite effect with 2 apps running on startup not having their tray icon shown. |
|
I did a few experiments and I found the problematic part.
is triggered as it's not yet ready. For me just turning the if to always be false is enough but I understand that in some cases it can have bad consequences so it's not a solution. |
Gnome Shell enables the extensions before the desktop itself is ready to accept widgets. This can result in errors. In the case of appindicator, it results in applications that are launched during startup not showing an indicator because, when the extension tries to add the icon to the Gnome Shell interface, it still isn't ready for that, thus failing. But other applications, launched after the Shell has completed the startup process, will work fine.
This patch fixes this by checking in the
enable()method if Gnome Shell is still starting up, in which case it waits until thestartup-completesignal is triggered before continuing.Fix #235